Skip to content

docs: list amp as a built-in target in the architecture and diagnostics references - #758

Merged
ScriptedAlchemy merged 3 commits into
mainfrom
docs/amp-builtin-target-references
Sep 8, 2026
Merged

docs: list amp as a built-in target in the architecture and diagnostics references#758
ScriptedAlchemy merged 3 commits into
mainfrom
docs/amp-builtin-target-references

Conversation

@ScriptedAlchemy

@ScriptedAlchemy ScriptedAlchemy commented Sep 8, 2026

Copy link
Copy Markdown
Owner

Closes #755. Closes #756.

Amp shipped as a built-in adapter in #729, but two documentation contracts still described the
four-adapter registry that preceded it, and both feed public pages.

website/docs/{en,zh}/guide/concepts/architecture.mdx (hand-written)

  • amp joins the planner table, naming adapters/amp.ts and its pinned
    adapters/capabilities/amp-0.0.0-20260907001852-gf348fed.json.
  • The createDefaultRegistry prose now includes amp and says what registration order is not:
    planComposite sorts the selection before planning, and the composite identity is the sorted
    name join. portable stays the default; plugin stays a non-target.
  • One new paragraph on what makes Amp architecturally different — a generated index.js
    PluginAPI factory under .amp/plugins/{plugin}/ that registers Skills and callbacks through
    the API, and skill-scoped MCP (exactly one Amp Skill for a canonical server; a compiler-owned
    local server refused for want of a documented plugin-root placeholder). The per-surface
    evidence stays on the generated Hosts page, which the paragraph links, along with the
    author-facing capability map from docs: complete convention-first guides, targets, and capability navigation (EN/ZH) #754.
  • "Four planners, one merge" becomes five, and BundleIdentityHost is corrected to
    amp | claude | codex | cursor with the note that DoctorHost excludes Amp.

docs/diagnostics.md (the source the site's diagnostics reference is rendered from)

  • AB4100: the built-in registry statement and the recovery list both name amp.
  • The pre-Amp enumerations found by re-reading the file: the skill-host lowering preamble,
    AB3006 (Amp's builtin-tools / mcpServers extension fields and targets.amp), AB3008
    (Amp documents no Skill Markdown interpolation engine, so it belongs with Codex, Cursor, and
    portable), AB3009 (foreign-placeholder detection covers Amp documents too), AB3010 (Amp
    validates the Agent Skills frontmatter schema plus its own skill MCP rules), the composite-root
    section, AB4106, and AB6023. The stale "unified plugin target's shared document" clauses
    went with them.
  • docs/diagnostics.md line 239 keeps its four host-scoped config keys deliberately: the Amp
    adapter declares no configExtension, so claude, codex, cursor, portable is the
    complete list there, matching reference/configuration.mdx.

No generated page was hand-edited — website/docs/{en,zh}/reference/diagnostics.md is rendered
from docs/diagnostics.md at build time, and the built output was checked to confirm the
AB4100 row now lists all five targets in both locales.

New guard (packages/agent-bundle/tests/diagnostics-doc-targets.test.ts): the AB4100 row's
trigger and recovery target lists are parsed out of docs/diagnostics.md and compared with
createDefaultRegistry().names(), so the next built-in adapter fails a test instead of silently
drifting the public reference. Falsified before landing: removing amp from the row fails the
test with the target diff.

Docs and a test only — no publishable package source changed, so pnpm changeset status --since=origin/main requires nothing (skip-changeset).

Local gate

Run on f6b92e55a3, which contains current origin/main (361d27ff5e):

Command Result
pnpm build pass
pnpm typecheck pass
pnpm lint pass
pnpm test:unit pass — 4480 tests, 0 failures
pnpm docs:site:build pass — locale drift, diagnostics coverage, language parity, 0 broken links / 30105 anchors
npx rstest --config rstest.unit.config.ts run packages/agent-bundle/tests/diagnostics-doc-targets pass

Deslop

Deslop: claude-fable-5-1-thinking-xhigh, 2 edits — trimmed an eight-line doc comment in the
guard test that narrated issue history, and removed the stale "unified plugin target's shared
document" clauses the Amp rows inherited.

Self-review

Reviewer: claude-fable-5-1-thinking-xhigh, then gpt-5.6-sol-max on the fix commit.

Finding Disposition
Blocking: adding amp to the composite-root paragraph made it claim Amp reads the artifact directory as its plugin root and shares skills/, hooks/, mcp/, scripts/, bin/, INSTALL.md. Amp's root is the nested .amp/plugins/<plugin>/ directory, self-contained down to its own skills/ and hooks/, and install amp copies only that subdirectory Fixed in ac38b42ac7: the shared-root sentence now covers claude, codex, cursor, portable, and Amp gets its own exception sentence
Non-blocking: the guard test's doc comment read as issue history Fixed in ac38b42ac7
Non-blocking: the guard's `([a-z]+)` name pattern would miss a future hyphenated or numeric target name Dismissed for now: every registered name is lowercase alpha, and a name the pattern misses fails the equality assertion rather than passing silently
Non-blocking: for Amp, AB3010's Agent Skills schema sees only the six portable keys, so builtin-tools is not schema-validated at lowering Dismissed: the row says Amp is validated by the Agent Skills schema plus Amp's own skill MCP rules, which is what the code does; it does not claim builtin-tools is schema-checked

@changeset-bot

changeset-bot Bot commented Sep 8, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 54ba151

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 8, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-08T00:14:13.355295Z f6b92e5 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@pkg-pr-new

pkg-pr-new Bot commented Sep 8, 2026

Copy link
Copy Markdown
npm i https://pkg.pr.new/ScriptedAlchemy/agent-bundle@758
npm i https://pkg.pr.new/ScriptedAlchemy/agent-bundle/create-agent-bundle@758
npm i https://pkg.pr.new/ScriptedAlchemy/agent-bundle/rsc-markdown-stream@758
npm i https://pkg.pr.new/ScriptedAlchemy/agent-bundle/@agent-bundle/runtime@758

commit: ac38b42

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f6b92e55a3

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread docs/diagnostics.md
| `AB3008` | error | The Skill Markdown body uses a canonical token (`agent-bundle:token:arguments`, `agent-bundle:path:plugin-root`, `agent-bundle:path:plugin-data`, `agent-bundle:path:workspace-root`, `agent-bundle:token:session-identity`, `agent-bundle:token:skill-root`) or a host alias of one (`$ARGUMENTS`, `${CLAUDE_PLUGIN_ROOT}`, `${PLUGIN_ROOT}`, `${workspaceFolder}`, …), and the skill lowers to a host whose pinned Skill Markdown contract documents no interpolation placeholder: Codex, Cursor, portable, and the unified `plugin` target's shared portable document. Claude documents all six. One diagnostic per token per host: `Skill token "<token>" has no <host> Skill Markdown equivalent.` | Remove the token, restrict the skill to a host that documents it, or move the reference to a document that host interpolates. |
| `AB3009` | error | After token lowering, the `<host>` Skill Markdown body still contains a placeholder spelling another host's contract owns — for Codex, Cursor, and portable documents, one of Claude's `$ARGUMENTS`, `${CLAUDE_PLUGIN_DATA}`, `${CLAUDE_PLUGIN_ROOT}`, `${CLAUDE_PROJECT_DIR}`, `${CLAUDE_SESSION_ID}`, `${CLAUDE_SKILL_DIR}`; the message names the syntax. | Use canonical agent-bundle tokens so lowering emits only this host's documented placeholders. |
| `AB3010` | error | The lowered `<host>` skill frontmatter fails that host's pinned schema — `schemas/skill-hosts/claude-skill-frontmatter.schema.json` and `cursor-skill-frontmatter.schema.json` for Claude and Cursor, the Agent Skills frontmatter schema for Codex and portable: `Lowered <host> Skill document <field> <message>.` | Remove the unsupported field or restrict the skill to a host that documents it. |
| `AB3006` | error | Skill frontmatter declares a field that is not a portable Agent Skills field (`name`, `description`, `license`, `compatibility`, `allowed-tools`, `metadata`), not a typed host extension (Claude: `agent`, `argument-hint`, `arguments`, `background`, `context`, `disallowed-tools`, `effort`, `hooks`, `model`, `shell`, `user-invocable`, `when_to_use`; Cursor: `color`, `globs`, `icon`; Amp: `builtin-tools`, `mcpServers`; shared: `disable-model-invocation`, `paths`), and not the authoring key `targets`; or `targets` is not an object, carries a key other than `amp`, `claude`, `codex`, `cursor`, or holds an unknown key inside `targets.<host>` (including `targets.amp`, `targets.codex.interface`, `.policy`, `.dependencies`, and `.dependencies.tools[<n>]`). The message names the field path. | Move host-only fields into `targets.<host>` or a documented host key, or remove the unknown field. |

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Remove valid targets.amp from the invalid examples

For a Skill using the newly supported targets: { amp: ... } extension, this row now simultaneously lists amp as an accepted target key and names targets.amp as an example of an unknown field. skills/parse-ir.ts explicitly accepts amp and reports only unknown keys nested beneath it, so the generated diagnostics reference incorrectly implies that valid Amp configuration triggers AB3006; remove this example or replace it with a genuinely invalid nested key.

AGENTS.md reference: AGENTS.md:L139-L142

Useful? React with 👍 / 👎.

@ScriptedAlchemy
ScriptedAlchemy merged commit 0ef0436 into main Sep 8, 2026
5 checks passed
@ScriptedAlchemy
ScriptedAlchemy deleted the docs/amp-builtin-target-references branch September 8, 2026 00:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant